Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

record: added a function to convert "total hits" for Elasticsearch #260

Merged
merged 1 commit into from
Sep 21, 2020
Merged

record: added a function to convert "total hits" for Elasticsearch #260

merged 1 commit into from
Sep 21, 2020

Conversation

Garfield-fr
Copy link
Contributor

@Garfield-fr Garfield-fr commented Sep 10, 2020 •
edited
Loading

This function allows to convert the total hits to keep the compatibility between versions 6 and 7 of Elasticsearch.

Co-Authored-by: Bertrand Zuchuat bertrand.zuchuat@rero.ch

Why are you opening this PR?

  • Compatibility Elasticsearch v6 and v7

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Extracted translations?

sebdeleze
sebdeleze approved these changes Sep 11, 2020
View reviewed changes
Copy link

@sebdeleze sebdeleze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, but seems to be a little bit overkill to create a pipe which is not attended to be used in template. Moreover, the pipe is used in only one component, so I think it would be better to create a method in the component instead.

@Garfield-fr Garfield-fr force-pushed the zub-es7-count branch 2 times, most recently from 4bdd56c to bb3441e Compare September 11, 2020 07:43
@Garfield-fr Garfield-fr changed the title pipe: added a pipe to convert "total hits" for Elasticsearch record: added a function to convert "total hits" for Elasticsearch Sep 11, 2020
@Garfield-fr
Copy link
Contributor Author

Ok, but seems to be a little bit overkill to create a pipe which is not attended to be used in template. Moreover, the pipe is used in only one component, so I think it would be better to create a method in the component instead.

I changed this to a function.

@Garfield-fr Garfield-fr force-pushed the zub-es7-count branch 2 times, most recently from df0a7fb to d70e3bf Compare September 11, 2020 08:55
@Garfield-fr
record: added a function to convert "total hits" for Elasticsearch
26b5ff2 
This function allows to convert the total hits to keep the compatibility between versions 6 and 7 of Elasticsearch.

Co-Authored-by: Bertrand Zuchuat <bertrand.zuchuat@rero.ch>
@iGormilhit
Copy link

@Garfield-fr Less important than for rero-ils or rero-ils-ui, but still, I miss a short explanation on why this commit is useful. Thanks.

@jma
Copy link
Contributor

jma commented Sep 14, 2020

Ok, but seems to be a little bit overkill to create a pipe which is not attended to be used in template. Moreover, the pipe is used in only one component, so I think it would be better to create a method in the component instead.

Perhaps it is useful for ui packages. Maybe.

@Garfield-fr Garfield-fr merged commit 9c0f542 into rero:dev Sep 21, 2020
@Garfield-fr Garfield-fr deleted the zub-es7-count branch September 21, 2020 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebdeleze sebdeleze sebdeleze approved these changes

@zannkukai zannkukai zannkukai approved these changes

@jma jma Awaiting requested review from jma

@iGormilhit iGormilhit Awaiting requested review from iGormilhit

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Garfield-fr @iGormilhit @jma @sebdeleze @zannkukai